Repository navigation
fix(spa): announce the UsersPage load-failure to screen readers - #419
Conversation
UsersPage rendered its load-failure (error && users === null) as a bare `<p className="error">`, while every sibling screen — AuditPage, ReportsPage, ExportPage, StockPage, HistoryPage, SettingsPage — uses role="alert" for the same case. role="alert" is an implicit live region, so without it a screen-reader user who deep-links to /users and is refused hears the heading and then silence, while the visual user sees red text. Add role="alert" to match the siblings. Scope is the unambiguous load-failure only. The dialog-level and page-level error paragraphs are left as-is: per #389 those are a judgment call (a dialog error adjacent to its field may be better served by aria-describedby than by an alert), and belong in a separate pass. No unit test added: UsersPage.test.tsx documents (its "UsersPage load" block) that the mount-effect rejection branch cannot be asserted in this Vitest 3 + React 19 stack without a false-positive unhandled rejection. Per #389 the #385 Playwright ReadOnly deep-link spec is where this tightens to getByRole("alert"). typecheck clean; the 58 existing UsersPage tests pass. Closes #389. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LSkqgsC9Sj4arUdevUSJfJ
|
@codex please review. One-line a11y fix:
Generated by Claude Code |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Review-loop follow-up. The page-level error banner (shown only when no dialog is open: error && !creating && openUser === null && editUser === null && pwUser === null) was still a bare `<p className="error">`, while the sibling page-level banners announce — ProductsPage (error && !dialogOpen) and ExpensesPage both mark theirs role="alert". It is the same unambiguous page-level class as the load-failure at line 347, not a dialog-adjacent judgment call, so it gets role="alert" too. Still deliberately left bare: the dialog-scoped validation errors (create / assign / edit / password dialogs), which #389 flags as a per-field judgment call (aria-describedby may beat an alert). typecheck clean; 58 UsersPage tests pass. Refs #389. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LSkqgsC9Sj4arUdevUSJfJ
|
Ran an adversarial review loop (codex + three cheap-model reviewers across two rounds). Outcome: One real finding, fixed in Still deliberately deferred: the four dialog-scoped validation errors (create / assign / edit / password dialogs, ~399/484/501/541). Per #389 those are a per-field judgment call ( Verified: the two alerts are mutually exclusive (line 347 is an early return, so the 408–409 banner only renders once Generated by Claude Code |
Closes #389.
UsersPagerendered its load-failure (error && users === null) as a bare<p className="error">, while every sibling screen —AuditPage,ReportsPage,ExportPage,StockPage,HistoryPage,SettingsPage— usesrole="alert"for the same case.role="alert"is an implicit live region, so without it a screen-reader user who deep-links to/usersand is refused hears the heading and then silence, while the visual user sees red text. This addsrole="alert"to match the siblings.Diff
Scope
The unambiguous load-failure only. The dialog-level and page-level error paragraphs (
UsersPage.tsx~399, 408–409, 484, 501, 541) are left as-is: per #389 those are a judgment call — a dialog error adjacent to its field may be better served byaria-describedbythan by an alert — and belong in a separate pass.Testing
UsersPage.test.tsx's "UsersPage load" block documents that the mount-effect rejection branch cannot be asserted in this Vitest 3 + React 19 stack without a false-positive unhandled rejection. Adding one would reintroduce exactly the flakiness this repo avoids. Per a11y: UsersPage refuses a load with no role=alert, so the refusal is silent to screen readers #389 the SPA E2E smoke: Playwright harness + five persona flows over the #243 sim fixture #385 Playwright ReadOnly deep-link spec is where this tightens togetByRole("alert")for both routes.npm run typecheck(clean) and the 58 existingUsersPage.test.tsxtests (all pass).🤖 Generated with Claude Code
https://claude.ai/code/session_01LSkqgsC9Sj4arUdevUSJfJ
Generated by Claude Code